gh-117657: Make frame clearing thread-safe#120542
gh-117657: Make frame clearing thread-safe#120542Fidget-Spinner wants to merge 5 commits intopython:mainfrom
Conversation
Fidget-Spinner
changed the title
colesbury
left a comment
colesbury
left a comment
There was a problem hiding this comment.
What is the test case that triggered the TSAN error?
Objects/genobject.c
Outdated
| } | ||
|
|
||
| static void | ||
| gen_dealloc(PyGenObject *gen) |
There was a problem hiding this comment.
This doesn't seem right to me. tp_dealloc can't be called concurrently on an object.
There was a problem hiding this comment.
In this case the racing part is mainly _PyFrame_ClearExceptCode and setting the generator's flags. _PyEval_FrameClearAndPop can race with this as it also clears a generator, but instead it's done by ceval.c.
There was a problem hiding this comment.
I don't understand -- how can _PyFrame_ClearExceptCode race with gen_dealloc?
It looks like gen_dealloc is only called from the tp_dealloc handler. If an object is being destroyed, then no other thread has access to it (or there's a separate serious bug).
There was a problem hiding this comment.
Hmm I'm not too sure either. Let me revert this fix for now and try to hunt it down again.
I first saw this getting triggered spuriously on our main test suite (I can't find the test now). But the TSAN logs I posted here are for the test case I wrote in this PR. |
|
All commit authors signed the Contributor License Agreement. |
|
I think this is fixed on main by #131479 |
AA-Turner
removed
needs backport to 3.13
5 participants
Data race reported by TSAN on the test: